docs(ui): use storybook-i18n addon for locale switching#2125
docs(ui): use storybook-i18n addon for locale switching#2125cylewaitforit wants to merge 1 commit intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
cylewaitforit
force-pushed
the
sb-storybook-i18n
branch
from
556fd5c to
542b245
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdded Storybook i18n support: appended Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.storybook/preview.ts (1)
22-26: Avoidanyfor the i18n instance and narrow before callingsetLocale.Line 22 and Lines 92-96 currently bypass type checks; a narrow interface here keeps the locale bridge safe and explicit.
💡 Suggested refactor
-let currentI18nInstance: any = null +type StorybookI18nInstance = { + setLocale: (locale: string) => void | Promise<void> +} +let currentI18nInstance: StorybookI18nInstance | null = null @@ - currentI18nInstance = this.$i18n + currentI18nInstance = + this.$i18n && typeof this.$i18n.setLocale === 'function' + ? (this.$i18n as StorybookI18nInstance) + : null @@ - if (locale && this.$i18n) { - this.$i18n.setLocale(locale) + if (locale && currentI18nInstance) { + void currentI18nInstance.setLocale(locale) }As per coding guidelines, "Ensure you write strictly type-safe code, for example by ensuring you always check when accessing an array value by index".
Also applies to: 91-96
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a584aa74-dd82-4f31-b2eb-5c2635be10b0
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
.storybook/main.ts.storybook/preview.tsapp/components/Link/Link.stories.tspackage.jsonpnpm-workspace.yaml
| // Subscribe to locale changes from storybook-i18n addon (once, outside decorator) | ||
| let currentI18nInstance: any = null | ||
| addons.getChannel().on('LOCALE_CHANGED', (newLocale: string) => { | ||
| if (currentI18nInstance) { | ||
| currentI18nInstance.setLocale(newLocale) | ||
| } | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Storybook 10 using storybook/preview-api, is channel.off(eventName, listener)the supported way to avoid duplicate listeners during HMR afterchannel.on(eventName, listener)?
💡 Result:
Yes. In Storybook 10, addons.getChannel() returns a Node.js EventEmitter-compatible channel, so removing a listener with channel.off(eventName, listener) is part of the supported API surface. [1]
Storybook’s own addon examples use the exact pattern to avoid duplicate subscriptions (subscribe with on, then unsubscribe with off in cleanup), e.g. channel.on(EVENTS.REQUEST, handleRequest) paired with channel.off(EVENTS.REQUEST, handleRequest). [2]
Also, Storybook’s @storybook/preview-api testing utilities model the channel with an explicit off(eventId, listener) method, reinforcing that off is the intended unsubscribe call. [3]
Important: you must pass the same function reference to off that you passed to on (i.e., don’t inline a new lambda each HMR pass). [2]
Sources: [1] (storybook.js.org) [2] (storybook.js.org) [3] (tessl.io)
Citations:
- 1: https://storybook.js.org/docs/addons/addons-api
- 2: https://storybook.js.org/addons/storybook-addon-apollo-client
- 3: https://tessl.io/registry/tessl/npm-storybook--preview-api/8.6.0/files/docs/testing-simulation.md
De-register the locale listener before re-registering it to avoid duplicate handlers during HMR.
Line 23 adds a module-scope LOCALE_CHANGED listener; on preview HMR reloads this can stack handlers and trigger repeated setLocale calls. Use channel.off() with the same function reference to clean up before re-subscribing.
💡 Suggested fix
-let currentI18nInstance: any = null
-addons.getChannel().on('LOCALE_CHANGED', (newLocale: string) => {
+let currentI18nInstance: any = null
+const channel = addons.getChannel()
+const onLocaleChanged = (newLocale: string) => {
if (currentI18nInstance) {
currentI18nInstance.setLocale(newLocale)
}
-})
+}
+channel.off?.('LOCALE_CHANGED', onLocaleChanged)
+channel.on('LOCALE_CHANGED', onLocaleChanged)Also consider typing currentI18nInstance to improve type safety.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Subscribe to locale changes from storybook-i18n addon (once, outside decorator) | |
| let currentI18nInstance: any = null | |
| addons.getChannel().on('LOCALE_CHANGED', (newLocale: string) => { | |
| if (currentI18nInstance) { | |
| currentI18nInstance.setLocale(newLocale) | |
| } | |
| }) | |
| // Subscribe to locale changes from storybook-i18n addon (once, outside decorator) | |
| let currentI18nInstance: any = null | |
| const channel = addons.getChannel() | |
| const onLocaleChanged = (newLocale: string) => { | |
| if (currentI18nInstance) { | |
| currentI18nInstance.setLocale(newLocale) | |
| } | |
| } | |
| channel.off?.('LOCALE_CHANGED', onLocaleChanged) | |
| channel.on('LOCALE_CHANGED', onLocaleChanged) |
cylewaitforit
force-pushed
the
sb-storybook-i18n
branch
from
542b245 to
c1f5529
Compare
1 participant
🔗 Linked issue
Related: #1964
🧭 Context
This adds the storybook-i18n for locale switching in the npmx storybook.
📚 Description
This allows stories that have been configured with a translation to be viewed with different translations.